fix: a footnote reference may wrap, and labels normalize before lookup - #269
Merged
Conversation
Upstream ruled the two sides of a footnote label asymmetric: a REFERENCE may
contain a line break, a DEFINITION marker may not. A reference that a text
editor has wrapped still binds, because the label is normalized before lookup,
while the definition marker stays a single-line construct the block parser can
find without scanning ahead.
This repo had taken the strict reading on both sides. The definition half was
right and stays; the reference half is reverted, and the missing piece behind
it is added: labels were never normalized at all here.
Normalization is the actual divergence, and it is wider than the newline.
Measured against djot.js 0.3.2 before this change, with `[^a b]: foo` as the
definition:
reference djot.js djot-php (before)
[^a b] binds does not bind
[^a<TAB>b] binds does not bind
[^ a ] binds does not bind
[^a<NL>b] binds* does not bind
* on djot.js main; 0.3.2 rejects only this one, which is the case
the upstream issue is about.
Link reference definitions in this parser already normalized (BlockParser's
`[label]: url` branch), and so did the reference side of a link, which is why
the same shapes bind for links and not for footnotes. The footnote path simply
never got it.
`StringUtil::normalizeLabel()` is djot.js's `normalizeLabel` character for
character - `trim()` plus `[ \t\r\n]+` to one space - rather than PHP's `\s`,
which also covers form feed and vertical tab. The difference is observable: on
`[t][a<FF>b]` with `[a b]: url`, djot.js leaves the reference unresolved and a
`\s`-based normalizer binds it. The two existing link-side call sites do use
`\s` and therefore still diverge on that character; that is pre-existing and
left alone here rather than folded into a footnote fix.
One consequence worth stating, because the rendered output changes shape. A
definition marker that IS wrapped stays a paragraph, and the paragraph is then
inline-parsed like any other - so its own `[^a` / `b]` is now a valid, and
usually unresolved, reference rather than literal text. The definition still
does not register, which is the part that matters.
The AST carries the normalized label, matching djot.js, so a wrapped reference
reports its undefined-footnote warning under the label it resolved to.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #269 +/- ##
============================================
- Coverage 92.41% 92.41% -0.01%
- Complexity 3673 3674 +1
============================================
Files 109 109
Lines 10421 10424 +3
============================================
+ Hits 9631 9633 +2
- Misses 790 791 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows the upstream ruling in jgm/djot.js#146: the two sides of a footnote label are deliberately asymmetric.
PR 267 here took the strict reading on both sides. The definition half was right and stays; the reference half is reverted, and the piece that makes it work is added.
The real gap was normalization, not the newline
Footnote labels were never normalized in this parser at all. Measured against the djot.js reference implementation, version 0.3.2, before this change, with
[^a b]: fooas the definition:[^a b][^a<TAB>b][^ a ][^a<NEWLINE>b]Only the last row is what the upstream issue is about; the first three were already divergent and nothing here covered them.
Link references did not have this problem, which is why it stayed invisible:
BlockParser's[label]: urlbranch normalizes the definition, and the inline side normalizes the reference. The footnote path had neither.Both are added now, so the two sides meet on the same key:
And the definition marker still does not cross a line ending:
No footnote is registered; the marker line stays a paragraph.
Character class
StringUtil::normalizeLabel()is djot.js'snormalizeLabelcharacter for character -trim()plus[ \t\r\n]+collapsed to one space - rather than PHP's\s, which also covers form feed and vertical tab. That difference is observable rather than academic: given[a b]: url,djot.js leaves the reference unresolved, a
\s-based normalizer binds it.The two existing link-side call sites do use
\sand still diverge on that one character. Pre-existing, and deliberately not folded into a footnote fix - happy to do it as a follow-up if you want the parity.One shape changes appearance
A definition marker that IS wrapped stays a paragraph, and a paragraph is inline-parsed like any other - so its own
[^a/b]is now a valid (and usually unresolved) reference rather than literal text. The definition still does not register, which is the part that matters, but the rendered paragraph shows a note marker where it previously showed the raw source.Notes
FootnoteLabelIsSingleLineTestis replaced byFootnoteLabelNormalizationTest: same definition-side coverage, plus the three line endings on the reference side and the whitespace variants on both sides.